xenstore size limits

 * Documents the existing 4kby size limit on xenstore message payloads
 * Causes xs.c in libxenstore to fail locally rather than violating
   said limit (which is good because xenstored kills the client
   connection if it's exceeded).
 * Introduces some limits on path lengths in xenstored.  I trust
   no-one is using path lengths >2kby.  This is good because currently
   a domain client can create a 4kby relative path that the dom0 tools
   cannot access since they'd have to specify the somewhat longer
   absolute path.
 * Removes uses of the host's PATH_MAX (!)

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

diff --git a/docs/misc/xenstore.txt b/docs/misc/xenstore.txt
index e0ad8f9b5232110dec91b830096e3727ca67eece..90632863dfa0225a8c848e55a95c7f6aac099e3d 100644 (file)
--- a/docs/misc/xenstore.txt
+++ b/docs/misc/xenstore.txt
@@ -38,7 +38,9 @@ The permitted character for paths set is ASCII alphanumerics and plus
 the four punctuation characters -/_@ (hyphen slash underscore atsign).
 @ should be avoided except to specify special watches (see below).
 Doubled slashes and trailing slashes (except to specify the root) are
-forbidden.  The empty path is also forbidden.
+forbidden.  The empty path is also forbidden.  Paths longer than 3072
+bytes are forbidden; clients specifying relative paths should keep
+them to within 2048 bytes.  (See XENSTORE_*_PATH_MAX in xs_wire.h.)
 
 
 Communication with xenstore is via either sockets, or event channel
@@ -56,6 +58,20 @@ order and must use req_id (and tx_id, if applicable) to match up
 replies to requests.  (The current implementation always replies to
 requests in the order received but this should not be relied on.)
 
+The payload length (len field of the header) is limited to 4096
+(XENSTORE_PAYLOAD_MAX) in both directions.  If a client exceeds the
+limit, its xenstored connection will be immediately killed by
+xenstored, which is usually catastrophic from the client's point of
+view.  Clients (particularly domains, which cannot just reconnect)
+should avoid this.
+
+Existing clients do not always contain defences against overly long
+payloads.  Increasing xenstored's limit is therefore difficult; it
+would require negotiation with the client, and obviously would make
+parts of xenstore inaccessible to some clients.  In any case passing
+bulk data through xenstore is not recommended as the performance
+properties are poor.
+
 
 ---------- Xenstore protocol details - introduction ----------
 

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 825d834e37d5e1462fd0df56b30f8e983b6189d8..acf6dd3918f256c68be62edf5c63a1de914b9059 100644 (file)
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -672,6 +672,9 @@ bool is_valid_nodename(const char *node)
        if (strstr(node, "//"))
                return false;
 
+       if (strlen(node) > XENSTORE_ABS_PATH_MAX)
+               return false;
+
        return valid_chars(node);
 }
 
@@ -1281,7 +1284,7 @@ static void handle_input(struct connection *conn)
                if (in->used != sizeof(in->hdr))
                        return;
 
-               if (in->hdr.msg.len > PATH_MAX) {
+               if (in->hdr.msg.len > XENSTORE_PAYLOAD_MAX) {
                        syslog(LOG_ERR, "Client tried to feed us %i",
                               in->hdr.msg.len);
                        goto bad_client;

diff --git a/tools/xenstore/xenstored_watch.c b/tools/xenstore/xenstored_watch.c
index 72927fa9c7ebd130a73d44b2c2157fa66786d3db..8e3e4f2b61c1f21ecc90fea3c004fa7b18794a82 100644 (file)
--- a/tools/xenstore/xenstored_watch.c
+++ b/tools/xenstore/xenstored_watch.c
@@ -125,6 +125,10 @@ void do_watch(struct connection *conn, struct buffered_data *in)
 
        if (strstarts(vec[0], "@")) {
                relative = false;
+               if (strlen(vec[0]) > XENSTORE_REL_PATH_MAX) {
+                       send_error(conn, EINVAL);
+                       return;
+               }
                /* check if valid event */
        } else {
                relative = !strstarts(vec[0], "/");

diff --git a/tools/xenstore/xs.c b/tools/xenstore/xs.c
index faa7e5c80f406143fe6aee15e0129c091c8317bd..a8152577980867fe692d58fcb229ca862460d2b5 100644 (file)
--- a/tools/xenstore/xs.c
+++ b/tools/xenstore/xs.c
@@ -319,6 +319,11 @@ static void *xs_talkv(struct xs_handle *h, xs_transaction_t t,
        for (i = 0; i < num_vecs; i++)
                msg.len += iovec[i].iov_len;
 
+       if (msg.len > XENSTORE_PAYLOAD_MAX) {
+               errno = E2BIG;
+               return 0;
+       }
+
        ignorepipe.sa_handler = SIG_IGN;
        sigemptyset(&ignorepipe.sa_mask);
        ignorepipe.sa_flags = 0;

diff --git a/tools/xenstore/xsls.c b/tools/xenstore/xsls.c
index cd8e3a9dacf57a5050dba1afc69866c613cfd62b..337e87cc5ba6a980f0a4a332a710025f5957f615 100644 (file)
--- a/tools/xenstore/xsls.c
+++ b/tools/xenstore/xsls.c
@@ -8,7 +8,7 @@
 #include <sys/ioctl.h>
 #include <termios.h>
 
-#define STRING_MAX PATH_MAX
+#define STRING_MAX XENSTORE_ABS_PATH_MAX+1024
 static int max_width = 80;
 static int desired_width = 60;
 static int show_whole_path = 0;

diff --git a/xen/include/public/io/xs_wire.h b/xen/include/public/io/xs_wire.h
index 927ed8c944e8809903d6015b3485091a2d513b6b..3994b11fdfd75f179dcb0cf894c240d26d0e4ea5 100644 (file)
--- a/xen/include/public/io/xs_wire.h
+++ b/xen/include/public/io/xs_wire.h
@@ -108,6 +108,13 @@ struct xenstore_domain_interface {
     XENSTORE_RING_IDX rsp_cons, rsp_prod;
 };
 
+/* Violating this is very bad.  See docs/misc/xenstore.txt. */
+#define XENSTORE_PAYLOAD_MAX 4096
+
+/* Violating these just gets you an error back */
+#define XENSTORE_ABS_PATH_MAX 3072
+#define XENSTORE_REL_PATH_MAX 2048
+
 #endif /* _XS_WIRE_H */
 
 /*